I have a WIP branch in #736 which takes a similar approach of providing a custom BIO_METHOD implementation, but it accepts a Ruby object that provide IO-like non-blocking read/write methods: #read_nonblock(integer, exception: false) and #write_nonblock(string, exception: false). Since I was expecting the main use case to be nested TLS, this felt like the most natural choice as the underlying IO interface.

Since we're just changing the BIO associated with the SSLSocket instance, we don't need to touch the I/O implementation in functions such as ossl_ssl_read_internal. The custom BIO interface will never return SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE, and thus the I/O operation will complete immediately after the call to e.g. SSL_read.

Wouldn't that convert all methods on SSLSocket blocking, including #{read,write,accept,connect}_nonblock? I feel support for these non-blocking methods is important, as many existing users currently rely on them to implement high-level timeout.

• Don't touch the implementation of the different SSLSocket I/O methods in ossl_ssl.c: #read, #write, #connect, #accept etc.

Some changes will be necessary there for proper error handling. Because Ruby exceptions are implemented using longjmp in C extensions, failing to handle them properly can corrupt OpenSSL's internal state or cause memory leaks.

If the underlying BIO raises an exception, we must temporarily catch it, allow OpenSSL to clean up its internal state, wait for SSL_read() to return, and then re-raise the exception from SSLSocket#sysread. Unfortunately, the ~10 different callbacks settable on SSLContext complicate this because they can be called at different timings. #736 is currently blocked because it likely doesn't handle all edge cases correctly yet (and I haven't had time to review them carefully, though I've been hoping to finish some time).

Wouldn't that convert all methods on SSLSocket blocking, including #{read,write,accept,connect}_nonblock? I feel support for these non-blocking methods is important, as many existing users currently rely on them to implement high-level timeout.

Indeed, this implementation would change the behavior to blocking, at least when using an IO-like object for the underlying BIO. However, for the custom BIO option (providing a read and a write proc), I have just added support for returning :wait_readableand:wait_writable`, which makes it possible to use the non-blocking API. Test case: https://github.com/ruby/openssl/pull/1000/changes#diff-9b8fe96fbb75f887aaa0cb69b982c886e041707496f37d8d6a5b443d92a5c347R2504

I understand the importance of supporting the non-blocking API. At the same time, having everything implemented as non-blocking is preventing other use cases, such as better integration with a fiber scheduler, or using io_uring for the actual I/O. Allowing a blocking BIO method is also an opportunity for achieving better performance, especially in conjunction with io_uring.

If the underlying BIO raises an exception, we must temporarily catch it, allow OpenSSL to clean up its internal state, wait for SSL_read() to return, and then re-raise the exception from SSLSocket#sysread.

I have included a test case for raising an exception from within the BIO method, that seems to be working correctly. However, this may need more testing. https://github.com/ruby/openssl/pull/1000/changes#diff-9b8fe96fbb75f887aaa0cb69b982c886e041707496f37d8d6a5b443d92a5c347R2460

Unfortunately, the ~10 different callbacks settable on SSLContext complicate this because they can be called at different timings. #736 is currently blocked because it likely doesn't handle all edge cases correctly yet (and I haven't had time to review them carefully, though I've been hoping to finish some time).

Looking at the code in ossl_ssl.c, it might be interesting to remove the calls to rb_protect in the different callback implementations, and instead wrap functions such as ossl_start_ssl with a single rb_protect. After looking more closely, this is more complicated than I first thought.

I'm closing this PR as it doesn't provide a good solution for supporting non-blocking I/O.

I don't think we should throw this out just because it doesn't support non-blocking.

If the only use case for non-blocking IO is users implementing timeouts, that's already better handled by IO#timeout.

As an aside, I personally think read_nonblock and write_nonblock was a big mistake for a user facing interface. So I don't think we should encourage it or use it as a standard internal interface. For example, using exceptions for waiting was an extremely poor design choice.

I agree the non-blocking API certainly complicates things but now it's just something we need to deal with. Maybe a solution would be to disable the non-blocking methods when a custom BIO is used? I'm open to suggestions, it's not clear to me how to proceed with this PR.

@HoneyryderChuck since you're behind the original issue #731 would you like to give your take on this PR? Can this solve your specific use case?

@noteflakes sorry for the late reply, haven't had much time to come back to the pending openssl related threads.

I like that your proposal is quite simple. Still, it seems that, in order to get non-blocking behaviour, I'd have to run on top of a fiber scheduler that hooks on the write/read blocking APIs, and that's unfortunately not the lowest common denominator in ruby. My original request had nothing to do with that though, I justed wanted to enable HTTPS proxy mode in httpx, the HTTP client library I maintain. And while it works much better (since some changes done last year) unde fiber schedulers, it's still mostly used without them, and its multiple request API relies on its own internal event loop, which relies on the nonblocking read/write APIs. That means that, for the most common use case, @rhenium 's work fits the most common case much better for me.

I still think that your angle of allowing a "bring your own BIO method" could be something worthwhile, particularly because in your case, you're targeting a liburing based fiber scheduler, so ideally openssl should accomodate that optionally for the cases of openssl sockets used under a thread running your scheduler; shipping your own BIO method on top of your fiber scheduler like a way to address it.

As an aside, I personally think read_nonblock and write_nonblock was a big mistake for a user facing interface.

I can understand the argument, but I think it's too late. Most (all?) network libs in ruby use the nonblocking variants. I haven't made a lot of use of IO.timeout= yet, so I'm not sure how much of an alternative it is, but on a first glance, it'd break with higher level non-fiber-scheduler-based event loops. From a pragmatic point of view, supporting the non-blocking variants here seems like a fraction of the work it'd take migrating the whole ecosystem to blocking APIs again while building fiber-scheduler variants which would be built on top of completeness-based I/O (not all OS/versions support them?) and convincing the community to use them primarily.

FWIW I'd like an openssl library built with more ruby, with a more direct layer of integration with libopenssl and friends. Most of the public interface maps to C extensions, and there's a lot of "callbacks" to CRuby API in between calls to the SSL API, which leaves some optimizations on the table, or reusing code in jruby impossible, but also make things such as implementing your own BIO method quite cumbersome. That's why I've trying to reimplement (most of) the ASN1 module in plain Ruby (something I have to get back to), and certainly something that I believe makes @rhenium patch much harder to validate (there's a lot of CRuby API in that BIO method, which makes it hard to consider the edge cases around error handling). Just my thoughts.

I still think that your angle of allowing a "bring your own BIO method" could be something worthwhile, particularly because in your case, you're targeting a liburing based fiber scheduler, so ideally openssl should accomodate that optionally for the cases of openssl sockets used under a thread running your scheduler; shipping your own BIO method on top of your fiber scheduler like a way to address it.

Fortunately this is pretty easy, and that's actually what I spent the last couple of days doing:

digital-fabric/uringmachine#3

From a pragmatic point of view, supporting the non-blocking variants here seems like a fraction of the work it'd take migrating the whole ecosystem to blocking APIs again while building fiber-scheduler variants which would be built on top of completeness-based I/O (not all OS/versions support them?) and convincing the community to use them primarily.

Maybe this got lost between the comments, but this PR already includes support for non-blocking I/O when using a custom BIO. 6c1db60

certainly something that I believe makes @rhenium patch much harder to validate (there's a lot of CRuby API in that BIO method, which makes it hard to consider the edge cases around error handling). Just my thoughts.

Regarding error handling, please correct me if I'm wrong, but it seems to me that this might actually be a non-issue, at least in most circumstances. If any callback raises an exception without rescuing it, or if any I/O method fails with a SystemCallError, or an SSLError, from the point of view of the application the way to deal with this would be to simply close the connection and optionally retry. So in that sense can we say that the SSL connection state post-exception is irrelevant? If there is a need to be able to recover an SSL connection after an exception, can you give an example?

I justed wanted to enable HTTPS proxy mode in httpx

You can already do this using a pipe, just in case you wanted something that's working today.

IO.timeout= yet, so I'm not sure how much of an alternative it is, but on a first glance, it'd break with higher level non-fiber-scheduler-based event loops

It works fine with or without a fiber scheduler.